Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design document for Y-cable API support for multiple vendors #757

Merged
merged 14 commits into from
Aug 30, 2021

Conversation

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@vdahiya12 vdahiya12 requested a review from jleveque March 12, 2021 02:38
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Copy link
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As comments. I will wait to review further until we close on these items.

doc/y_cable/design_doc.md Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
@jleveque jleveque changed the title Design document for Y cable API support for multiple vendors Design document for Y-cable API support for multiple vendors Mar 29, 2021
@andywongarista
Copy link
Contributor

@jleveque @vdahiya12 For the sonic_sfp refactor I'm planning on doing something similar with the vendor to module mapping, whereby a xcvr vendor that releases a product (say, some QSFP module) with use of vendor-specific fields in the EEPROM can create a vendor-specific module and have that be accessible via the mapping after determining the module's vendor name and part number.

Something I'd like some clarification on is what "Y cable vendor" means in this doc - my initial understanding was that there is a distinct group of y cable vendors (distinct from xcvr vendors) that provide the actual y cables and have some some sort of spec that (at least with Credo) uses unused xcvr EEPROM pages for y cable logic, and that y cable vendors would be implementing the API described in this doc to control their particular y cable. It looks like the vendor name and part number mentioned come from reading the EEPROM of the xcvr connected to the physical port of TORA or TORB, so to me it seems like this is really the transceiver vendor info that we're retrieving. If it is then there might be some overlap with what I'm doing.

@jleveque
Copy link
Contributor

Something I'd like some clarification on is what "Y cable vendor" means in this doc - my initial understanding was that there is a distinct group of y cable vendors (distinct from xcvr vendors) that provide the actual y cables and have some some sort of spec that (at least with Credo) uses unused xcvr EEPROM pages for y cable logic, and that y cable vendors would be implementing the API described in this doc to control their particular y cable. It looks like the vendor name and part number mentioned come from reading the EEPROM of the xcvr connected to the physical port of TORA or TORB, so to me it seems like this is really the transceiver vendor info that we're retrieving. If it is then there might be some overlap with what I'm doing.

@andywongarista: Your assumptions are correct. We need to check the vendor and part number from the EEPROM to determine which Y-cable API module to load. There may indeed be some overlap here. However, for the purposes of your refactoring, I would think that you're more interested in the transceiver identifier fields (in order to determine the proper spec) rather than vendor/part number fields. Can you explain why the vendor and part number fields are pertinent to your work?

@andywongarista
Copy link
Contributor

@jleveque Yes, checking the transceiver identifier is the main concern (at least with how things are now). The vendor/part number fields are also pertinent because of the hypothetical example that I mentioned; the specs define certain regions in the EEPROM which are vendor-specific. Currently we aren't interested in any such regions in the Sfp code, but perhaps in the future we will be. If a vendor were to provide a module extending some class representing a base spec with logic relating to these vendor-specific regions, then we would want a means of making use of that vendor's module, and my current thinking is that the vendor/part number fields would then become relevant in a similar manner to how they're used in this doc.

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@jleveque
Copy link
Contributor

@jleveque Yes, checking the transceiver identifier is the main concern (at least with how things are now). The vendor/part number fields are also pertinent because of the hypothetical example that I mentioned; the specs define certain regions in the EEPROM which are vendor-specific. Currently we aren't interested in any such regions in the Sfp code, but perhaps in the future we will be. If a vendor were to provide a module extending some class representing a base spec with logic relating to these vendor-specific regions, then we would want a means of making use of that vendor's module, and my current thinking is that the vendor/part number fields would then become relevant in a similar manner to how they're used in this doc.

Gotcha. Yeah, in that case, you'd pretty much be using the vendor/part number fields in a very similar manner to what we're doing here, so there would indeed be some overlap. Do you intend to add this infrastructure as part of the refactor, even though, as you said, we currently aren't interested in vendor-specific regions in the SFP code? If so, then we can try to think of the best way to avoid duplicating code.

@andywongarista
Copy link
Contributor

@jleveque Yes, checking the transceiver identifier is the main concern (at least with how things are now). The vendor/part number fields are also pertinent because of the hypothetical example that I mentioned; the specs define certain regions in the EEPROM which are vendor-specific. Currently we aren't interested in any such regions in the Sfp code, but perhaps in the future we will be. If a vendor were to provide a module extending some class representing a base spec with logic relating to these vendor-specific regions, then we would want a means of making use of that vendor's module, and my current thinking is that the vendor/part number fields would then become relevant in a similar manner to how they're used in this doc.

Gotcha. Yeah, in that case, you'd pretty much be using the vendor/part number fields in a very similar manner to what we're doing here, so there would indeed be some overlap. Do you intend to add this infrastructure as part of the refactor, even though, as you said, we currently aren't interested in vendor-specific regions in the SFP code? If so, then we can try to think of the best way to avoid duplicating code.

Yes, I'm planning to create a mapping file anyways to link transceiver identifiers with the appropriate spec and I figured it would be best to make this mapping somewhat future-proof by making it easy to add vendor-specific mappings if they ever become needed. I suspect there will be a way to avoid having two mappings.

What I'm still confused about is the meaning behind checking vendor name and part number from EEPROM to load the right Y-cable API module. The vendor name and part number read from EEPROM correspond to the transceiver vendor. What that means to me is that if a Credo Y-cable was connected to a QSFP from Vendor A, then potentially a different Y-cable API module would need to be loaded if the QSFP was swapped with one from Vendor B, even if we were still using the same Credo Y-cable. Thus, the Y-cable vendor (Credo) is not what's distinguishing the different Y-cable API modules, and so the "Y-cable vendor" wording used in the doc seems misleading. I may be missing something here conceptually so please do clarify if I am.

@jleveque
Copy link
Contributor

What I'm still confused about is the meaning behind checking vendor name and part number from EEPROM to load the right Y-cable API module. The vendor name and part number read from EEPROM correspond to the transceiver vendor. What that means to me is that if a Credo Y-cable was connected to a QSFP from Vendor A, then potentially a different Y-cable API module would need to be loaded if the QSFP was swapped with one from Vendor B, even if we were still using the same Credo Y-cable. Thus, the Y-cable vendor (Credo) is not what's distinguishing the different Y-cable API modules, and so the "Y-cable vendor" wording used in the doc seems misleading. I may be missing something here conceptually so please do clarify if I am.

With the Y-cables, the actual transceivers are active components and are fixed to the cable; they can not be replaced in the field. the entire cable along with all three transceivers is one unit and must be replaced as a whole. Therefore, the vendor/part number of the EEPROM in these cables will reliably tell us that the cable is a Y-cable. This is one difference from standard QSFP cables/transceivers.

@andywongarista
Copy link
Contributor

What I'm still confused about is the meaning behind checking vendor name and part number from EEPROM to load the right Y-cable API module. The vendor name and part number read from EEPROM correspond to the transceiver vendor. What that means to me is that if a Credo Y-cable was connected to a QSFP from Vendor A, then potentially a different Y-cable API module would need to be loaded if the QSFP was swapped with one from Vendor B, even if we were still using the same Credo Y-cable. Thus, the Y-cable vendor (Credo) is not what's distinguishing the different Y-cable API modules, and so the "Y-cable vendor" wording used in the doc seems misleading. I may be missing something here conceptually so please do clarify if I am.

With the Y-cables, the actual transceivers are active components and are fixed to the cable; they can not be replaced in the field. the entire cable along with all three transceivers is one unit and must be replaced as a whole. Therefore, the vendor/part number of the EEPROM in these cables will reliably tell us that the cable is a Y-cable. This is one difference from standard QSFP cables/transceivers.

Ah, thanks for the clarification. In that case the infrastructure I'm adding for supporting different vendors should handle the Y-cable vendors as well. I'll provide an explanation for how this might work in the revised proposal and we can discuss later if it makes sense. The initial focus of the sonic_sfp refactoring won't be on integrating y cable functionality with the new design, so we'll need to sync up when that happens but I don't think the integration will be too troublesome.

doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
doc/y_cable/design_doc.md Outdated Show resolved Hide resolved
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
- Vendors can place their implementations in this format

```
sonic_platform_common/sonic_y_cable/<vendor>/<module>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't we have vendor specific y_cable implementation in sonic-buildimage repo instead of platform-common repo (which shouldn't be having any changes/implementation which are platform specific)? For eg Sfpbase.py resides in platform-common whereas its realization/implementation reside in platform specific :- platform/broadcom/sonic-platform-modules-dell/z9264f/sonic_platform/sfp.py i.e in sonic-buildimage repo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, platform is really a platform in a sense we have different SKU's, related configs etc, here these are just cables. Even though we could do something similar seems an overkill where it is easy to maintain everything in one place.

vdahiya12 added a commit to sonic-net/sonic-platform-common that referenced this pull request Jul 29, 2021
…I support for multiple vendors (#186)

Description
For multiple Y-Cable vendor support once we do have a mapping from vendor/part number to the appropriate Y-Cable module to load, we need to map appropriate port to a module as well. This PR adds definition for base abstract class.

Motivation and Context
Basically, the key idea is once we have a port identified as being of a certain vendor and it has been identified to load a certain module, it is then needed to call the correct module/API on each port each time we call the API on the port

it is required to maintain this mapping in memory since xcvrd does not want to read/parse this y_cable_vendor_mapping.py file again and again each time we call the Y-Cable API

Also note that the module loaded might change during xcvrd running lifetime since cable/transceiver can be changed from one vendor to another. So we need to take this into consideration as well

Proposed Solution for this
Each module of the Y-Cable vendor can be a class (of each transceiver type) and all we need to do is instantiate the objects of these classes as class instances and these objects will provide the interface of calling the API's for the appropriate vendor Y-Cable.

This instantiation will be done inside xcvrd, when xcvrd starts

These objects basically emulate Y-Cable instances and whatever action/interaction needs to be done with the Y-Cable the methods of these objects would provide that

each vendor in their implementation can inherit from a base class where there will be definitions for all the supported capabilities of the Y-Cable.

for vendors the recommended approach in case their subclass implementation does not support a method, is to set the method equal to None. This differentiates it from a method they forgot to implement. Then, the calling code should first check if the method is None before attempting to call it.

Design document for the support is sonic-net/SONiC#757

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
qiluo-msft pushed a commit to sonic-net/sonic-platform-common that referenced this pull request Aug 3, 2021
…I support for multiple vendors (#186)

Description
For multiple Y-Cable vendor support once we do have a mapping from vendor/part number to the appropriate Y-Cable module to load, we need to map appropriate port to a module as well. This PR adds definition for base abstract class.

Motivation and Context
Basically, the key idea is once we have a port identified as being of a certain vendor and it has been identified to load a certain module, it is then needed to call the correct module/API on each port each time we call the API on the port

it is required to maintain this mapping in memory since xcvrd does not want to read/parse this y_cable_vendor_mapping.py file again and again each time we call the Y-Cable API

Also note that the module loaded might change during xcvrd running lifetime since cable/transceiver can be changed from one vendor to another. So we need to take this into consideration as well

Proposed Solution for this
Each module of the Y-Cable vendor can be a class (of each transceiver type) and all we need to do is instantiate the objects of these classes as class instances and these objects will provide the interface of calling the API's for the appropriate vendor Y-Cable.

This instantiation will be done inside xcvrd, when xcvrd starts

These objects basically emulate Y-Cable instances and whatever action/interaction needs to be done with the Y-Cable the methods of these objects would provide that

each vendor in their implementation can inherit from a base class where there will be definitions for all the supported capabilities of the Y-Cable.

for vendors the recommended approach in case their subclass implementation does not support a method, is to set the method equal to None. This differentiates it from a method they forgot to implement. Then, the calling code should first check if the method is None before attempting to call it.

Design document for the support is sonic-net/SONiC#757

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
vdahiya12 added a commit to sonic-net/sonic-platform-daemons that referenced this pull request Aug 25, 2021
…for calling Y-Cable API's inside xcvrd (#197)

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

Description
This PR integrates vendor specific class objects inside xcvrd for Y-Cable API's to be called.
Detailed designed document can be found sonic-net/SONiC#757

Motivation and Context
Basically xcvrd now has an interface for Y-Cable to interact with PMON docker and HOST
which can be uniform across all vendors. As part of this refactor, we will be moving towards a model where only xcvrd interacts with the cables/transceivers, and host-side processes will communicate with xcvrd rather than with the devices directly with Y-Cable.

How Has This Been Tested?
Ran the changes on Arista7050cx3 switch, making changes inside the container.

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
vdahiya12 added a commit to sonic-net/sonic-utilities that referenced this pull request Aug 25, 2021
…tation from vendors (#1722)

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

What I did
this PR adds support for cli refactor which is done for abstract class implementation of muxcable which is provided by vendors.
detailed design doc can be found sonic-net/SONiC#757

How I did it
Added the changes in muxcable.py in show and config .

How to verify it
add unit-tests as well as run on a Arista-7050cx3 switch, run unit tests.

no change in outputs, just the functionality has changed.
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
vdahiya12 added a commit to vdahiya12/sonic-utilities that referenced this pull request Aug 25, 2021
…tation from vendors (sonic-net#1722)

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

What I did
this PR adds support for cli refactor which is done for abstract class implementation of muxcable which is provided by vendors.
detailed design doc can be found sonic-net/SONiC#757

How I did it
Added the changes in muxcable.py in show and config .

How to verify it
add unit-tests as well as run on a Arista-7050cx3 switch, run unit tests.

no change in outputs, just the functionality has changed.
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
vdahiya12 added a commit to vdahiya12/sonic-utilities that referenced this pull request Aug 25, 2021
…tation from vendors (sonic-net#1722)

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

What I did
this PR adds support for cli refactor which is done for abstract class implementation of muxcable which is provided by vendors.
detailed design doc can be found sonic-net/SONiC#757

How I did it
Added the changes in muxcable.py in show and config .

How to verify it
add unit-tests as well as run on a Arista-7050cx3 switch, run unit tests.

no change in outputs, just the functionality has changed.
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
vdahiya12 added a commit to vdahiya12/sonic-platform-daemons that referenced this pull request Aug 25, 2021
…for calling Y-Cable API's inside xcvrd (sonic-net#197)

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

Description
This PR integrates vendor specific class objects inside xcvrd for Y-Cable API's to be called.
Detailed designed document can be found sonic-net/SONiC#757

Motivation and Context
Basically xcvrd now has an interface for Y-Cable to interact with PMON docker and HOST
which can be uniform across all vendors. As part of this refactor, we will be moving towards a model where only xcvrd interacts with the cables/transceivers, and host-side processes will communicate with xcvrd rather than with the devices directly with Y-Cable.

How Has This Been Tested?
Ran the changes on Arista7050cx3 switch, making changes inside the container.

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
vdahiya12 added a commit to vdahiya12/sonic-platform-daemons that referenced this pull request Aug 25, 2021
Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

Description
This PR integrates vendor specific class objects inside xcvrd for Y-Cable API's to be called.
Detailed designed document can be found sonic-net/SONiC#757

Motivation and Context
Basically xcvrd now has an interface for Y-Cable to interact with PMON docker and HOST
which can be uniform across all vendors. As part of this refactor, we will be moving towards a model where only xcvrd interacts with the cables/transceivers, and host-side processes will communicate with xcvrd rather than with the devices directly with Y-Cable.

How Has This Been Tested?
Ran the changes on Arista7050cx3 switch, making changes inside the container.

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
vdahiya12 added a commit to sonic-net/sonic-platform-daemons that referenced this pull request Aug 26, 2021
…for calling Y-Cable API's inside xcvrd (#197) (#213)

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com
Merging this in 202012 branch because there was a merge conflict in original PR
#197

Description
This PR integrates vendor specific class objects inside xcvrd for Y-Cable API's to be called.
Detailed designed document can be found sonic-net/SONiC#757

Motivation and Context
Basically xcvrd now has an interface for Y-Cable to interact with PMON docker and HOST
which can be uniform across all vendors. As part of this refactor, we will be moving towards a model where only xcvrd interacts with the cables/transceivers, and host-side processes will communicate with xcvrd rather than with the devices directly with Y-Cable.

How Has This Been Tested?
Ran the changes on Arista7050cx3 switch, making changes inside the container.

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com
vdahiya12 added a commit to sonic-net/sonic-utilities that referenced this pull request Aug 26, 2021
…tation from vendors (#1722) (#1782)

Merging this in 202012 branch because there was a merge conflict in original PR while cherry-picking
#1722

#### What I did
this PR adds support for cli refactor which is done for abstract class implementation of muxcable which is provided by vendors.
detailed design doc can be found sonic-net/SONiC#757

#### How I did it
Added the changes in muxcable.py in show and config . 

#### How to verify it
add unit-tests as well as run on a Arista-7050cx3 switch, run unit tests. 

no change in outputs, just the logic has changed.
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@vdahiya12 vdahiya12 merged commit a7d1067 into sonic-net:master Aug 30, 2021
malletvapid23 added a commit to malletvapid23/Sonic-Utility that referenced this pull request Aug 3, 2023
…tation from vendors (#1722)

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

What I did
this PR adds support for cli refactor which is done for abstract class implementation of muxcable which is provided by vendors.
detailed design doc can be found sonic-net/SONiC#757

How I did it
Added the changes in muxcable.py in show and config .

How to verify it
add unit-tests as well as run on a Arista-7050cx3 switch, run unit tests.

no change in outputs, just the functionality has changed.
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants